Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Efficient MXNet sampling in the multinomial distribution #15311

Merged
merged 4 commits into from
Jun 22, 2019
Merged

Efficient MXNet sampling in the multinomial distribution #15311

merged 4 commits into from
Jun 22, 2019

Conversation

zixuanweeei
Copy link
Contributor

@zixuanweeei zixuanweeei commented Jun 21, 2019

Description

This PR has solved the problem of low efficiency to sample from the multinomial distribution, especially the distribution with a large number of possible outcomes. As it was described in #15231, it costs ~44 s using mx.nd.random.multinomial, while np.random.multinomial costs ~0.019 s. After our optimization, it decreases to ~0.054 s.

Feature changes

New features

  • Efficient sampling strategy for the multinomial distribution using binary search.
  • Use double type to define some cumulative variables for high precision with very small probabilities.
  • Reserve the CDF array for sampling many times without duplicated addition operations.

Performance

We compared the time costs of mx.nd.random.multinomial on branch master(b8b352d) and our PR shown in the table below. The result shows that the time cost of the original sampling strategy rises much more rapidly than that of our PR with the increasing number of outcomes.

Number of outcomes Master(b8b352d) Costs (ms) PR(6c3c49b) Costs (ms)
10 0.125 0.066
100 0.270 0.058
1000 0.729 0.130
10000 62.717 0.996
100000 5372.306 12.125
1000000 - 173.534

Comments

@pengzhao-intel @ciyongch @TaoLv Please help me refine this PR and have some review on it. Thanks.

Check list

  • Passed code style checking (cpplint).
  • Unit test passed.
  • Code is well-documented.

@pengzhao-intel
Copy link
Contributor

@reminisce @wkcn for review.

@TaoLv
Copy link
Member

TaoLv commented Jun 21, 2019

Nice work! @zixuanweeei
@chinakook @stu1130 You might be interested.

Copy link
Member

@wkcn wkcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. LGTM. Thank you!

Copy link
Contributor

@stu1130 stu1130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brillant! LGTM

@wkcn wkcn added the pr-awaiting-merge Review and CI is complete. Ready to Merge label Jun 21, 2019
@chinakook
Copy link
Contributor

Thanks, LGTM!

@wkcn wkcn merged commit e6fad30 into apache:master Jun 22, 2019
@wkcn
Copy link
Member

wkcn commented Jun 22, 2019

Merged. Thanks for your contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants